Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a test on Number related locale #9807

Closed
wants to merge 2 commits into from
Closed

Add a test on Number related locale #9807

wants to merge 2 commits into from

Conversation

rommni
Copy link
Contributor

@rommni rommni commented Nov 5, 2019

Like discussed on qooxdoo/qooxdoo-compiler#587 add test to number related locale value.

Testing fr locale allow to test that the locale linked to defaultNumberingSystem are loaded and not just the first one declared in cldr files

return;
}

var Number = qx.locale.Number;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overwriting the native Number object is certainly legal, but is it a good idea? Maybe use a different symbol?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cboulanger: do you mean the name of the test class? It’s not qx.locale.Number, it’s qx.test.locale.Number!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! Sorry you mean Number! Yes. this should be avoided.

Copy link
Contributor Author

@rommni rommni Nov 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum effectively i took the code from the qx.test.locale.Date, i will change that.

@hkollmann
Copy link
Member

I am missing the change of compile.json for the french locale?

@rommni
Copy link
Contributor Author

rommni commented Nov 6, 2019

@hkollmann after more investigating the change wasn't needed, the fact is that for testing i added the qxl.testtapper package and was running the test with it and this one was compiled using compile.json settings and so was lacking the fr locale, however the framework config settings contains the french locale.

@hkollmann
Copy link
Member

Can you add your testtapper compile.json as compileTest.json?

@rommni
Copy link
Contributor Author

rommni commented Nov 6, 2019

@hkollmann of course i can, do you want me to push my Manifest.json and qx-lock.json with testtapper add too? Maybe only the qx-lock? Currently i include in compile.json testtapper app only my qx.test.locale.Number class, should i let this or change it?

@hkollmann
Copy link
Member

I suggest a project in testgui from where the testrunner can run. Your test is not working in the python toolchain. With this test project we are able to run the tests with the compiler tool chain.

@cboulanger
Copy link
Contributor

cboulanger commented Nov 7, 2019

@hkollmann Do you mean the test breaks the python toolchain (because the tests seem to pass), or it simply doesn't run in the generator-based toolchain. Because if the latter is the case, it doesn't really matter, does it?

@hkollmann
Copy link
Member

It's not running. But i would like to archive the test project as a base for changing the test procedure

Copy link
Member

@hkollmann hkollmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the test project

@rommni
Copy link
Contributor Author

rommni commented Nov 8, 2019

sorry a little busy currently, i'll looking that as soon as i have the time ;)

@oetiker oetiker added this to In progress in 6.0 Release Dec 17, 2019
@oetiker oetiker moved this from In progress to To do in 6.0 Release Dec 17, 2019
@oetiker oetiker removed this from To do in 6.0 Release Dec 17, 2019
@cboulanger
Copy link
Contributor

Hi @rommni , any chance you can revisit and finalize this PR? Thank you.

@rommni
Copy link
Contributor Author

rommni commented Mar 30, 2020

Hi @cboulanger sorry i totally forgotten this one, of course i will look at this (trying to do that in the week)

@rommni
Copy link
Contributor Author

rommni commented Apr 9, 2020

Just a message to tell i haven't forget this but just not enough time these last days doing that as soon i can

@hkollmann
Copy link
Member

Testing is completely reworked

@hkollmann
Copy link
Member

repos is gone,

@hkollmann hkollmann closed this Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants